-
Couldn't load subscription status.
- Fork 14.8k
KAFKA-19784: KIP-1227: Expose rack ID in MemberDescription and ShareMemberDescription #20691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Hi @jiafu1115 . Thanks for the PR. This would be a tiny change to a public interface and that means it needs an accompanying KIP. It would be a small KIP and not at all controversial I think. Would you like to create the KIP or would you like me to do this? Also, note that |
Hi. @AndrewJSchofield Thanks for your review and reminder. I will create one KIP for the workflow and check the shared group code according to your suggestion. |
|
@AndrewJSchofield Update: KIP https://cwiki.apache.org/confluence/display/KAFKA/KIP-1227%3A+Expose+Rack+ID+in+MemberDescription+and+ShareMemberDescription created and I also updated the change for share group. We can wait one week before the vote to collect feedbacks though it is one tiny change. |
|
Update: I fixed the compile/check style issues and squashed the commits. |
|
Hi @AndrewJSchofield . The KIP has been accepted, Thanks a lot! So we can move forward now. I built the client SDK with the branch and tested it with the following cases today. I think the results meet my expectations. I’m sharing them with you for your review! Thanks! Case 1kafka 4.1 Case 2kafka 4.1 Case 3kafka3.9 Case 4kafka 4.1-Share Group Case 5kafka 4.1-Share Group |
Signed-off-by: stroller <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates. Just very minor comments this time. Will be ready to merge soon.
tools/src/test/java/org/apache/kafka/tools/consumer/group/ShareGroupCommandTest.java
Outdated
Show resolved
Hide resolved
clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/apache/kafka/clients/admin/internals/DescribeConsumerGroupsHandlerTest.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/apache/kafka/clients/admin/internals/DescribeConsumerGroupsHandlerTest.java
Outdated
Show resolved
Hide resolved
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java
Outdated
Show resolved
Hide resolved
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java
Outdated
Show resolved
Hide resolved
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java
Outdated
Show resolved
Hide resolved
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java
Outdated
Show resolved
Hide resolved
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java
Outdated
Show resolved
Hide resolved
clients/src/main/java/org/apache/kafka/clients/admin/MemberDescription.java
Outdated
Show resolved
Hide resolved
e565e04 to
cd4511c
Compare
|
@AndrewJSchofield Done for all the changed. Help to review it again. Really appreciate it! |
Signed-off-by: stroller <[email protected]>
|
@AndrewJSchofield Thank you so much for your thorough review and continuous follow-up — really appreciate it! |
|
@jiafu1115 Thanks for your first KIP. |
Hi Team:
Currently, the AdminClient’s describeConsumerGroups API returns a
MemberDescription that does not include rack information, even though
the underlying ConsumerGroupDescribeResponse protocol already supports a
rackId field. This causes users to be unable to retrieve member rack
information through the Admin API.
Rack information is crucial for:
In addition, StreamsGroupMemberDescription already includes the rackId,
so adding it here would make the behavior more consistent.
BTW, I have currently implemented our AZ/Rack analysis using a
workaround — passing the rack information into the clientId field and
parsing it afterward.
So I propose this change with tiny code change to support this feature.
After the change. We can easy to get all the rack info for every
consumer by the API:
Admin#describeConsumerGroups(java.util.Collection<java.lang.String>)Thanks for review!
Reviewers: Andrew Schofield [email protected]